RDKEMW-23354: mics occasionally muted at boot - #268
Conversation
Reason for change: Sometimes at boot the privacy setting is checked before the input/output routes are completed so ctrlm cane default to privacy enabled when it should not Test Procedure: go to settings and confirm that privacy mode is off, microphones are enabled. Reboot and check that privacy is still disabled. Testing will require a lot of reboots, as the original report said this issue might happen only 1 in 10 boots Risks: low Priority: P1 Signed-off-by: Jason Thomson <jason_thomson@comcast.com>
There was a problem hiding this comment.
Pull request overview
This PR targets an intermittent boot-time condition where privacy/mic state can be evaluated before audio input/output routing is fully established, causing the system to incorrectly default to privacy-enabled (mics muted). It refactors privacy synchronization into a dedicated helper and triggers that refresh after route updates.
Changes:
- Introduces
ctrlm_voice_t::voice_update_privacy()as a shared helper for privacy-state refresh logic. - Moves the privacy refresh trigger to occur after route updates (
voice_sdk_update_routes()), aligning timing with completed input/output routing. - Removes the earlier privacy-refresh block from initial configuration flow so it happens at the more appropriate lifecycle point.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/voice/ctrlm_voice_obj.h | Declares new voice_update_privacy() helper on the base voice object. |
| src/voice/ctrlm_voice_obj.cpp | Implements voice_update_privacy() and removes the prior inline privacy refresh during initial configuration. |
| src/voice/ctrlm_voice_obj_generic.cpp | Calls voice_update_privacy() after xrsr_route() to re-evaluate privacy once routes are updated. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.
Suppressed comments (2)
src/voice/ctrlm_voice_obj.cpp:3732
voice_update_privacy()readsdevice_statuswithout takingdevice_status_semaphore, and the mismatch resolution can never take effect because it branches onvoice_is_privacy_enabled()(ctrlm state) and then callsvoice_privacy_enable/disable(), which will early-return when ctrlm already matches that value. If the intent is to refresh ctrlm privacy based on the VSDK (likevoice_power_state_change()does), make VSDK the source of truth and only then update ctrlm/DB, while keeping semaphore usage consistent.
void ctrlm_voice_t::voice_update_privacy() {
if(this->local_mic) {
// Read privacy mode state from the DB in case power cycle lost HW GPIO state
if(this->device_status[CTRLM_VOICE_DEVICE_MICROPHONE] & CTRLM_VOICE_DEVICE_STATUS_DISABLED) {
XLOGD_INFO("voice is disabled, skip privacy");
src/voice/ctrlm_voice_obj_generic.cpp:400
voice_update_privacy()is called even whenxrsr_route(routes)fails. Since the purpose is to refresh privacy after a successful route update, keep the privacy refresh in the success path so failures don’t trigger additional VSDK privacy reads/writes during an already-failed route update.
//Updating routes means updating inputs and outputs, so refresh the privacy setting in case inputs changed
this->voice_update_privacy();
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.
Suppressed comments (2)
src/voice/ctrlm_voice_obj_generic.cpp:400
voice_update_privacy()is called even whenxrsr_route(routes)fails. If routes/inputs/outputs were not successfully updated, refreshing privacy based on the VSDK at this point can persist an incorrect state (and the call intent in the comment is no longer true). Consider returning early on route failure and only refreshing privacy after a successfulxrsr_routecall.
if(!xrsr_route(routes)) {
XLOGD_ERROR("failed to set routes");
}
//Updating routes means updating inputs and outputs, so refresh the privacy setting in case inputs changed
this->voice_update_privacy();
src/voice/ctrlm_voice_obj.cpp:3738
voice_update_privacy()usesvsdk_is_privacy_enabled(), which defaults privacy to ON whenxrsr_privacy_mode_getfails. Persisting that default into ctrlm/DB here can incorrectly mute mics (and make the boot issue worse if the privacy query is still transiently failing). It’s safer to only update ctrlm/DB when the VSDK privacy query succeeds, otherwise leave the existing ctrlm/DB privacy state unchanged.
// Refresh ctrlm/DB privacy state from the VSDK after route/input updates.
const bool privacy_vsdk = this->vsdk_is_privacy_enabled();
const bool privacy_ctrlm = this->voice_is_privacy_enabled();
if(privacy_vsdk != privacy_ctrlm) {
privacy_vsdk ? this->voice_privacy_enable(false) : this->voice_privacy_disable(false);
}
Reason for change: Sometimes at boot the privacy setting is checked before the input/output routes are completed so ctrlm cane default to privacy enabled when it should not
Test Procedure: go to settings and confirm that privacy mode is off, microphones are enabled. Reboot and check that privacy is still disabled. Testing will require a lot of reboots, as the original report said this issue might happen only 1 in 10 boots
Risks: low
Priority: P1